Skip to content

fix(objectql): the value-shape detail prescribes the key rename, not the missing-pair type error - #16078

Merged
zhuangjianguo merged 1 commit into
mainfrom
claude/issue-15490-value-shape-detail-prefers-unrecognized-keys
Sep 6, 2026
Merged

fix(objectql): the value-shape detail prescribes the key rename, not the missing-pair type error#16078
zhuangjianguo merged 1 commit into
mainfrom
claude/issue-15490-value-shape-detail-prefers-unrecognized-keys

Conversation

@claude

@claude claude Bot commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

Fixes #15490

An operator running os migrate value-shapes over real legacy location data was told Invalid input: expected number, received undefined and left to derive a rename that edit distance cannot reach — while LocationValueSchema had already built the prescription (latitude -> lat) and thrown it away.

All evidence below is from commit 290dfc53f.

The defect

A value-shape rejection was read positionally, parsed.error.issues[0], at two sites — the card names one:

site (re-derived on this branch; the card's anchors had moved) code
record-validator.ts:795 const detail = parsed.error.issues[0]?.message ?? 'invalid value shape'; — the write path's warn-first / strict branch
record-validator.ts:946 return parsed.error.issues[0]?.message ?? 'invalid value shape'; — inside the exported valueShapeViolation, which the scan imports

zod reports per-member issues before the object-level unrecognized_keys one, so on a value whose keys were renamed the actionable message sorts last. Fixing only the second site would have left the first emitting the identical wrong prescription — which is exactly the drift the comment at :840 already worries about, so both now share one reader.

Deliverable 1 — the sweep, done first, and it changed what shipped

The card fenced its own suggestion: confirm the positional read does not mislead the other structured-JSON classes before changing it. Every class the two readers cover was measured through the real valueSchemaFor, with a battery per class (missing members, undeclared keys, both at once, wrong base type):

type family can emit unrecognized_keys? can co-occur with invalid_type? issues[0] when it does
json structured-JSON no no
composite structured-JSON no no
repeater structured-JSON no no
record structured-JSON no no
location structured-JSON yes yes invalid_type
address structured-JSON yes yes invalid_type
vector structured-JSON no no
lookup / master_detail / user / tree reference no no
file / image / avatar / video / audio file-reference no no

16 classes measured; 2 can emit unrecognized_keys at all. Only location and address are backed by a key-closed object schema. The reference and file-reference classes are strings, composite / record / repeater / vector are open records and arrays, json is z.unknown(), and the one deliberately loose object shape (FileValueSchema) never refuses a key.

So preferring unrecognized_keys is a no-op for the other fourteen — it cannot change a single character of their diagnosis, which is what makes it safe as a blanket rule rather than merely untested against them. And both classes it does reach curate an aliases map, so the undeclared key is the half carrying the prescription in both.

The sweep refuted the reason address looked immune

The triage read was that address declares every member optional, so no invalid_type can sort ahead of the undeclared-key issue. Measured, that holds only for the missing-member route. A wrong-typed declared member still emits invalid_type, and it still sorts first:

address  { street: 5, postal_code: '98101' }
  issues[0] invalid_type       path=['street']  Invalid input: expected string, received number
  issues[1] unrecognized_keys  path=[]          ... Did you mean `postal_code` -> `postalCode`? ...

So the defect reaches address too; the contrast case that made it look location-only just happened to carry no type error. That case is now pinned.

End-to-end, through the real exported scanValueShapes

case before after
A. legacy {latitude, longitude} location Invalid input: expected number, received undefined — no rename names latitude -> lat, longitude -> lng
B. {postal_code} address (the contrast that already worked) full rename prescription unchanged
C. all-clean control blocking=0, passed unchanged
D. address wrong-typed member + undeclared key Invalid input: expected string, received number names postal_code -> postalCode

blocking counts are identical before and after in every case.

Finishing the pin

scan-value-shapes.test.ts already used the exact value { geo: { latitude: 1, longitude: 2 } } and asserted only report.blocking === 1 — never detail. The fixture was there; only the assertion was missing, which is why no gate could see this. It now asserts the prescription, and names the string that was being shown instead so a regression to the positional read is visible.

Ablation (one shell; mutation proven on disk before measuring, restore proven after):

mutate both sites back to positional:
  injected positional reads = 2 (expect 2), remaining helper calls = 0 (expect 0)
  mutated blob d34e3809... != HEAD blob 08f474fd...      <- mutation confirmed on disk
green leg (fix in place)    exit=0    Test Files 1 passed (1)   Tests 9 passed (9)
red leg   (mutated)         exit=1    Test Files 1 failed (1)   Tests 2 failed | 7 passed (9)
    x #15490: an undeclared key outranks a same-value member error - on address too
        AssertionError: expected 'Invalid input: expected string, recei...' to contain '`postal_code` -> `postalCode`'
    x the scan counts exactly what strict mode rejects - one predicate, not two
        AssertionError: expected 'Invalid input: expected number, recei...' to contain '`latitude` -> `lat`'
restore: blob == HEAD blob AND `git diff HEAD` empty AND index clean
green leg (restored)        exit=0    Test Files 1 passed (1)   Tests 9 passed (9)
VERDICT: ABLATION VALID - the pin CAN see this defect.

The 7 tests that passed in both legs are every blocking count and every accept/reject assertion — reverting the fix moved no verdict, which is the same fact from the other direction.

No verdict moves

The same values are flagged, the same writes rejected or admitted, and the deployment gate opens on the same evidence. The helper is only reached after the parsed.success branch, so the accept/reject decision never consults it. Only operator-facing text changes: the scan finding's detail, the warn-first [value-shape] log line, and the invalid_value_shape error's detail under strict enforcement.

Clause-2: no — both limbs measured separately on @objectstack/objectql's built surface.

  • Does any exported symbol or signature move? No. Built at head, swapped both changed sources back to base c463d03e0, rebuilt, diffed dist/index.d.ts. With the content-hash chunk filename normalized (util-D3s8yRSa -> util-mkEsVIjj, which moves because implementation bytes changed), the only remaining difference is the JSDoc on ValueShapeFinding.detail. Declared-symbol sets are identical, 116 vs 116. valueShapeDetail is module-private and appears 0 times in the emitted .d.ts. The base rebuild was byte-identical to a snapshot taken before any edit, which confirms the base artifact. Restore proven byte-exact, and a final rebuild put dist/ back to the head artifact.
  • Is any request newly accepted or rejected? No, per the ablation's shared-pass set above.

Verification (all at 290dfc53f)

  • pnpm --filter @objectstack/objectql exec vitest run --maxWorkers=2 src/validation/scan-value-shapes.test.ts src/validation/record-validator.test.tsTest Files 2 passed (2), Tests 104 passed (104)
  • pnpm --filter @objectstack/objectql typecheck — exit 0, all three legs; check:test-typecheck: OK — 44 file(s) / 242 error(s) / 69 pinned signature(s) held (ledger unmoved)
  • pnpm lint (whole repo, eslint . --no-inline-config) — exit 0
  • Gate family re-derived after the final commit with node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack, provenance line checked: "gate list derived from the tree of 'objectstack-ai/objectstack' at commit 290dfc5". All 57 run: 55 exit 0; the 2 non-zero are both exit 3 = PREREQUISITE NOT MET, which each gate documents as "NOT a pass and NOT a finding":
    • check:dual-build-cjs-loads — reads built output for 54 packages with no dist/; its own --self-test passed 93 cases
    • check:type-check-debt — needs the whole workspace build closure. The targeted answer is the package typecheck above, green with its debt ledger unmoved

CI runs both with the full build.

Out of scope

packages/spec/src/data/default-value-shape.ts:120 carries a third positional issues[0] read producing a detail string. It is a different function on the defaultValue authoring path rather than the stored-value path this card governs, so it is not touched here. Filed separately as #16077, with the measurement.


Generated by Claude Code

…the missing-pair type error

A value-shape rejection was read positionally (`parsed.error.issues[0]`) at both
sites that produce the operator-facing detail: the write path's warn-first /
strict branch, and the exported `valueShapeViolation` the `os migrate
value-shapes` scan imports. zod reports per-member issues before the
object-level `unrecognized_keys` one, so on a value whose keys were RENAMED the
actionable message sorts last and was discarded — a `location` stored as
`{latitude, longitude}` reported `expected number, received undefined` while
`LocationValueSchema`'s curated `aliases` map had already built the rename
prescription that edit distance cannot reach.

Both readers now share one helper that prefers the undeclared-key issue when the
rejection carries one. Scoped by a sweep of every class the two readers cover:
only `location` and `address` are backed by a key-closed object schema, so only
they can emit `unrecognized_keys` at all and the preference is a no-op for the
other fourteen types. The sweep also refuted the reason `address` looked immune
— all-optional members rule out a missing-member type error, not a wrong-typed
declared one — so a pin now covers that class too.

No verdict moves: the same values are flagged and the same writes rejected.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ARYe3yQTQCUFm5qPYNgKaJ
@github-actions github-actions Bot added the size/m label Sep 5, 2026
@github-actions

github-actions Bot commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/objectql, touching 6 documentable anchor(s).

1 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:

  • content/docs/protocol/objectql/types.mdx (via unrecognized_keys (literal, a string literal in valueShapeDetail))

1 release-owned page(s) also name something this change touched. These are read-only:

  • content/docs/releases/v17.mdx (via unrecognized_keys (literal, a string literal in valueShapeDetail))

content/docs/releases/ is RELEASE-OWNED (AGENTS.md "Documentation Guardrails"): release
notes are written centrally at release time, and a code PR that edits them is the exact PR
that guardrail exists to stop. They are still audited — read-only. If one of them is actually
wrong, file an issue or open a dedicated docs-only PR; do not edit it here.

What this run could not see
  • the SDK route bridge reached 61 of 219 client-bound route-ledger rows — the other 158 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 158: 0 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 56 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 102 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.

Coarse fallback — 16 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json c2520416cff04575dd3def789dd04a5745e3912bpackageMentionDocs.

Which tree this was computed on

This run read content/docs from 9f904f92363123fb50f7540b7c627517cb79fc01 — the merge of head 290dfc53f88953aeb6efeb965476d19ca750962f into base c2520416cff04575dd3def789dd04a5745e3912b, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 9f904f92363123fb50f7540b7c627517cb79fc01 && git checkout 9f904f92363123fb50f7540b7c627517cb79fc01
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin c2520416cff04575dd3def789dd04a5745e3912b 290dfc53f88953aeb6efeb965476d19ca750962f && git checkout -B drift-repro c2520416cff04575dd3def789dd04a5745e3912b && git merge --no-ff 290dfc53f88953aeb6efeb965476d19ca750962f

node scripts/docs-audit/affected-docs.mjs --json c2520416cff04575dd3def789dd04a5745e3912b

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs c2520416cff04575dd3def789dd04a5745e3912b → pass the list as
args.docs, on the commit named under Which tree this was computed on.

@github-actions github-actions Bot added documentation Improvements or additions to documentation tests tooling labels Sep 5, 2026
@claude

claude Bot commented Sep 5, 2026

Copy link
Copy Markdown
Contributor Author

Docs Drift Check verdict — a positive hit, read properly

Read at this PR's head 290dfc53f, and read as a page rather than searched for the literal — the standing note that this page went UNLISTED for #11430 while being the page that diff falsified is exactly why.

content/docs/protocol/objectql/types.mdxCORROBORATED (and strengthened). Not falsified.

The falsifying shape is absent. The page nowhere asserts which parse issue a caller sees first, nowhere names issues[0], and nowhere mentions the detail field. The only ordering language on all 1306 lines is L405-406, about datetime text ordering staying chronological — unrelated.

The listing anchor is a false lead. The literal unrecognized_keys occurs once, L736, inside the relationship-field callout: "Both branches were once gated on reference_to — a key the spec REFUSES (FieldSchema answers unrecognized_keys for it, on any field type)". That is metadata authoring-key refusal, not a stored value, and not this layer. Untouched.

What the page does say about my layer — two passages, both about the author-facing prescription for a malformed structured-JSON value:

Both describe the prescription an author receives. My change moves them toward truth, not away from it, so the verdict is corroborated:

page's promise on base c463d03e0 at 290dfc53f
location: undeclared key latitude "refused by name" scan reported Invalid input: expected number, received undefined — names no key names latitude -> lat, longitude -> lng
address: postal_code "fails with a rename to postalCode" true only when no member is also wrong-typed true in both cases

The notable part: one of these lines was ALREADY falsified on origin/main, by the base code, not by this diff

The address sentence promises postal_code "fails with a rename to postalCode" without qualification. Measured on base, that held only for the fixture shape the page happens to show. Add a wrong-typed declared member and the promise broke:

address  { street: 5, postal_code: '98101' }        (base c463d03e0)
   detail -> "Invalid input: expected string, received number"     <- not a rename; the page's sentence was false here

This is the sweep's finding reaching the docs: all-optional members rule out a missing-member invalid_type, but not a wrong-typed declared one, which still sorted ahead of the undeclared-key issue. This PR closes that gap, so the page's unqualified sentence becomes true for the first time. That case is pinned in scan-value-shapes.test.ts.

No docs edit is owed, and none was made. The page states the rule my layer implements; the PR makes the implementation match the already-written rule. Editing it would be widening the PR for no gain.

content/docs/releases/v17.mdxUNTOUCHED, and correct. Not edited, no follow-up owed.

Read at L776-795 and L1344 (read-only, release-owned). Its unrecognized_keys anchor is the wire error catalog: "Routes that validate with Zod stopped leaking Zod's issue codes: ... unrecognized_keys becomes unknown_field, and ... a missing required property now reports required instead of the invalid_type Zod uses for it". That is a different layer — it maps Zod issue codes to public FieldErrorCode values on EnhancedApiError.fields. This PR changes neither which Zod issues exist nor any code mapping; it changes which issue's message text becomes the objectql value-shape detail. The invalid_value_shape code itself is unchanged. L1344 (ui as an unrecognized_keys error) is metadata authoring, also unrelated. Nothing there is wrong, so there is no separate docs issue to raise.

Scope

This was a read. Nothing in the dispatch moved: the sweep still came first and still gated the blanket rule (16 classes measured, 2 can emit unrecognized_keys at all), both call sites are fixed (record-validator.ts:795 and :946, re-derived), and the detail assertion in scan-value-shapes.test.ts is in and ablation-proven.


Generated by Claude Code

@zhuangjianguo
zhuangjianguo marked this pull request as ready for review September 6, 2026 00:04
@zhuangjianguo
zhuangjianguo added this pull request to the merge queue Sep 6, 2026
Merged via the queue into main with commit d61d6e3 Sep 6, 2026
35 checks passed
@zhuangjianguo
zhuangjianguo deleted the claude/issue-15490-value-shape-detail-prefers-unrecognized-keys branch September 6, 2026 01:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation size/m tests tooling

Projects

None yet

2 participants